V2: Expand IRI with AmSC Token, Keep Globus and Facility still in place (Beta, do not merge)#92
V2: Expand IRI with AmSC Token, Keep Globus and Facility still in place (Beta, do not merge)#92juztas wants to merge 9 commits into
Conversation
| "opentelemetry-instrumentation-fastapi>=0.60b1,<0.61b0", | ||
| "opentelemetry-exporter-otlp>=1.39.1,<1.40.0", | ||
| "globus-sdk>=4.3.1", | ||
| "PyJWT>=2.10.1", |
There was a problem hiding this comment.
Have you considered a higher level library like authlib? It might take care of some of the validation code in _decode_oidc_jwt. Take a look here: https://docs.authlib.org/en/stable/oauth2/resource-server/flask.html
There was a problem hiding this comment.
I have not until now. I think that would be a bigger lift for the codebase. Current IRI needs are basically to validate a signed JWT, check the issuer/aud/exp. I think Authlib provides a full set of server/client tooling. Do we need that?
There was a problem hiding this comment.
I think minimizing boilerplate is usually a good idea. Here are two options:
- don't know much about this one: https://fastapi-oidc.readthedocs.io/en/latest/
- the "standard": joserfc with:
- import keyset (jwks): https://jose.authlib.org/en/recipes/cheatsheet/#key-sets-jwks
- decode/validate the jwt: https://jose.authlib.org/en/recipes/cheatsheet/#decode-verify-token (where the 'key' param is the jwks from the prev. step)
|
|
||
| def _fetch_oidc_remote_state(discovery_uri: str) -> tuple[dict[str, Any], KeySet]: | ||
| """Fetch the OIDC discovery.""" | ||
| with httpx.Client(timeout=_DISCOVERY_TIMEOUT_SECONDS) as client: |
There was a problem hiding this comment.
Should this be async? I like the aiohttp library.
Update: httpx also supports async access
| return metadata, JsonWebKey.import_key_set(jwks_resp.json()) | ||
|
|
||
|
|
||
| def _load_oidc_remote_state(discovery_uri: str) -> tuple[dict[str, Any], KeySet]: |
There was a problem hiding this comment.
How about instead using caching on _fetch_oidc_remote_state, via:
from cachetools import cached, TTLCache
@cached(cache=TTLCache(maxsize=128, ttl=60))
|
Looks good thanks! I left some minor comments. |
No description provided.